Perf: Optimize pages loading (Filecache path like approach) - #2549
Conversation
5d10baa to
5e6b2e9
Compare
mejo-
left a comment
There was a problem hiding this comment.
Thanks a lot @Koc, this looks really promising 🤩
I have some comments, but I'm genuinely curious what you think about the comments.
If you do further changes to the PR, could you do them in separate fixup commits (and don't force-push changes to the existing commit for now) so it's easier to review your changes?
3c03b17 to
980fb42
Compare
|
We have green pipeline 🎉 |
0ac5ab9 to
6937cb2
Compare
max-nextcloud
left a comment
There was a problem hiding this comment.
Proposing a refactoring of the getPagesFromFolder / buildPageInfoTree functions.
I'm pretty sure your approach works as well. I think this refactoring would still clarify things a lot. Would you be up for it or shall I give it a try and open a PR against this one?
|
Just tested this locally and performance wise it's really great. Managed to handle a collective with 2k pages well. Became somewhat slow from 3k on. |
|
@max-nextcloud @mejo- WDYT about adding a composite index to |
|
@Koc there's the Might be interesting to confirm it's being used though. On my local instance requests for > 1k files were way below 1 second. That seems good enough. |
|
From my point of view the next step would be to fetch the pages for even larger collectives in multiple requests. |
|
@max-nextcloud heh we're using Postgres 😕 Why it's excluded? I will try to work on your other comments next week |
The latter seems to have some discussions on creating the index by hand to avoid the DBAL limitations. |
6937cb2 to
8806c19
Compare
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
795d7ac to
79f03d5
Compare
|
Thanks a lot for your patience and commitment @Koc |
|
Tracking behat failure in #2614 |
Issues have been adressed. Follow up issue created.
📝 Summary
This is alternative approach to #2390 that fixes same performance issue (closes #2380).
Benefits comparing to previous implementation:
So, we're just load all necessary pages via simple query
SELECT * FROM filecache WHERE storage_id = <storageId> AND path LIKE 'appdata_<instanceId>/collectives/<collectiveId>/%'🖼️ Screenshots
Collective with 390 pages with various nesting level
🚧 TODO
filecachetable tostorage_id, pathcolumns (but this requires extra PR to nextcloud/server) - see comment Perf: Optimize pages loading (Filecache path like approach) #2549 (comment)🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)🤖 AI (if applicable)